Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

PIP-211 [feat][tiered-storage] Introduce offload throttling, Part 1 #17485

Closed
wants to merge 18 commits into from
Closed

PIP-211 [feat][tiered-storage] Introduce offload throttling, Part 1 #17485

wants to merge 18 commits into from

Conversation

tjiuming
Copy link
Contributor

@tjiuming tjiuming commented Sep 6, 2022 •
edited
Loading

Fixes #17362

Motivation

Support offload throttling, to prevent offloading use up all the broker resources(CPU, network).

From the perspective of generality, we can't limit how many bytes write to tiered-storage, it will invade the corresponding implementations(filesystem, streaming, etc.). So we can only to limit how many bytes read from ML, this cannot absolutely limit the traffic per second.

Modifications

  1. Move TimeWindow.java、WindowWrap.java、TimeWindowTest.java from broker module to managed-ledger module.
  2. Add managedLedgerGlobalOffloadingPermitBytesPerSecond configuration, it works on the broker level.
  3. Add globalOffloadingPermitBytesPerSecond into ManagedLedgerConfig.

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API: (yes)
  • The schema: (no)
  • The default values of configurations: (no)
  • The wire protocol: (no)
  • The rest endpoints: (no)
  • The admin cli options: (no)
  • Anything that affects deployment: (no)

Documentation

Check the box below or label this PR directly.

Need to update docs?

  • doc-required
    (Your PR needs to update docs and you will update later)

  • doc-not-needed
    (Please explain why)

  • doc
    (Your PR contains doc changes)

  • doc-complete
    (Docs have been already added)

Matching PR in forked repository

PR in forked repository: tjiuming#11

@tjiuming
Copy link
Contributor Author

tjiuming commented Sep 6, 2022

@hangc0276 @codelipenghui PTAL

@tjiuming
Copy link
Contributor Author

tjiuming commented Sep 6, 2022

I'll create another PR to support restApi/cmd to modify offload policies after the PR merged

@tjiuming
Copy link
Contributor Author

tjiuming commented Sep 6, 2022

Tests to be completed.

@github-actions github-actions bot added the doc-required Your PR changes impact docs and you will update later. label Sep 6, 2022
@tjiuming tjiuming mentioned this pull request Sep 7, 2022
Open
2 tasks
@zymap zymap added type/feature The PR added a new feature or issue requested a new feature area/tieredstorage labels Sep 7, 2022
@zymap zymap added this to the 2.12.0 milestone Sep 7, 2022
@tjiuming tjiuming requested review from zymap and removed request for hangc0276, eolivelli, codelipenghui and mattisonchao September 7, 2022 17:37
@dao-jun
fix checkstyle and add Test
2b0aba0
@dao-jun
Merge branch 'master' into dev/offload_throttling
03082c7 
# Conflicts:
#	managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java
@codelipenghui
Copy link
Contributor

@tjiuming, Could you please create a proposal for this one? We will introduce a new mechanism, we need to consider how to operate and monitor the new feature.

@tjiuming
Copy link
Contributor Author

@tjiuming, Could you please create a proposal for this one? We will introduce a new mechanism, we need to consider how to operate and monitor the new feature.

@codelipenghui Yes, I'm re-designing the flow-controller, the current implementation will block read thread and will take effect to other ledger's reading. After that, I'll open a proposal

@github-actions
Copy link

The pr had no activity for 30 days, mark with Stale label.

@github-actions github-actions bot added the Stale label Oct 20, 2022
@dao-jun
FIX
0bbc195
@dao-jun
Merge branch 'master' into dev/offload_throttling
7a174c9 
# Conflicts:
#	managed-ledger/src/main/java/org/apache/bookkeeper/mledger/ManagedLedgerConfig.java
#	managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java
#	pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
@dao-jun
FIX
8ca776c
@tjiuming tjiuming changed the title [feat][tiered-storage] Introduce offload throttling PIP-211 [feat][tiered-storage] Introduce offload throttling, Part 1 Nov 24, 2022
@tjiuming tjiuming mentioned this pull request Nov 24, 2022
Open
@poorbarcode
Copy link
Contributor

Since we will start the RC version of 3.0.0 on 2023-04-11, I will change the label/milestone of PR who have not been merged.

  • The PR of type feature is deferred to 3.1.0
  • The PR of type fix is deferred to 3.0.1

So drag this PR to 3.1.0

@poorbarcode poorbarcode modified the milestones: 3.0.0, 3.1.0 Apr 10, 2023
@github-actions github-actions bot removed the Stale label Apr 11, 2023
@github-actions
Copy link

The pr had no activity for 30 days, mark with Stale label.

@github-actions github-actions bot added the Stale label May 12, 2023
@Technoboy- Technoboy- modified the milestones: 3.1.0, 3.2.0 Jul 31, 2023
@Technoboy- Technoboy- modified the milestones: 3.2.0, 3.3.0 Dec 22, 2023
@dao-jun dao-jun closed this Mar 29, 2024
@dao-jun dao-jun mentioned this pull request Mar 29, 2024
Open
15 tasks
@dao-jun
Copy link
Member

dao-jun commented Mar 29, 2024

Replaced by #22385

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zymap zymap Awaiting requested review from zymap

Assignees

@tjiuming tjiuming

Labels
area/tieredstorage doc-required Your PR changes impact docs and you will update later. ready-to-test Stale type/feature The PR added a new feature or issue requested a new feature
Projects
None yet
Milestone
3.3.0
Development

Successfully merging this pull request may close these issues.

[feature] Support offload throttling
6 participants
@tjiuming @codelipenghui @poorbarcode @dao-jun @zymap @Technoboy-